-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: remove util from test-repl-setprompt #3338
Conversation
Util is being used only for string templating. Inspired by #3324, this commit removes util from the test, and instead uses back ticks to do the templating. The commit also replaces the var declaration with const, and replaces the leading comma style used for variable declaration to match the style found in other tests.
LGTM if CI is OK. |
Good to see this kind of cleanup happening. LGTM so long as CI is green. |
these probably would have been easier to manage and review in a single PR and single commit, as it is we're going to end up with a fair bit of noise for what's really just a single change across multiple files |
Yes, @thealphanerd for changes like this where it is all very similar it would be preferred to keep it to a single PR & commit. :) |
@rvagg / @Fishrock123 thanks for the heads up. Will keep similar cleanup in a single PR in the future. |
@rvagg @Fishrock123 I was about to land this and the two related PRs, but before I do (or someone else does because I'm about to go to bed): Should I squash the three of them into a single commit? The only issue I can think of with doing that is that there will be three
|
@Trott feel free to take liberty in changing the commit title to better reflect all three living in one place |
(Alternative: Close these three PRs, ask @thealphanerd to cherry-pick the commits from the three PRs onto a clean branch, optionally squash, open a new PR. Only advantage I can think of to that is if it's important that there be only one |
I can do that before the am |
done :D |
this should go to LTS /cc @jasnell |
Util is being used only for string templating. Inspired by #3324, this commit removes util from the test, and instead uses back ticks to do the templating. The commit also replaces the var declaration with const, and replaces the leading comma style used for variable declaration to match the style found in other tests.